Add SSH certificate authentication for targets, issued by Vault - #2397

Will be happy to see this merged since it's the only deployment blocker for us due to security concerns.

Went through this again against the current head (1578fc67), plus a step back from line-level review to look at the design itself.

Real progress since the last round: the AWS static-credential gap is now a genuine, well-designed fix, AwsError::StaticCredentialsDisallowed rejects when no STS session token is present, correctly distinguishing static IAM keys from any temporary/workload-identity credential. And the cached Vault token is now wrapped in zeroize::Zeroizing instead of the old Secret<String>, closing the memory-zeroization gap from last round.

Three things from the last round are still open, each with a concrete fix.

Vault issuer errors reaching the SSH client are still truncated to 256 characters rather than sanitized by content, so a policy or role name can survive that length. The fix is a client_message()-style method on VaultError that returns a generic, category-level message ("Vault denied the certificate signing request," "Vault is currently unavailable") to the SSH client, with the full error logged server-side instead, and this isn't Vault-specific, ConnectionError::Aws falls through the same catch-all in session.rs today, so it's worth fixing as a shared mechanism rather than a one-off for this arm.

stub_vault.py has no request-shape validation for the AWS, Azure, or GCP login paths, only Kubernetes and AppRole get checked, so the suite can't catch a malformed request on three of the five methods, worth adding the same presence checks those two already get.

Also, test_aws_signs_the_global_endpoint_by_default is currently broken, confirmed by actually running it: it supplies static credentials with no session token, so StaticCredentialsDisallowed correctly rejects before Warpgate ever reaches Vault, and the test's own assertion crashes with an empty-list error since Vault is never contacted. Fix is small: add AWS_SESSION_TOKEN to that test's env the same way the sibling test does.

The bigger thing: stepping back from individual lines, there's a structural question worth resolving before this merges. Under the current design, Vault can't distinguish one target/session from another, role, principals, and key_id are all values Warpgate's own code asserts in the signing request, not anything Vault independently verifies. Under the model this replaces, compromising Warpgate's stored credentials was bounded by whatever was actually stored for actually-configured targets. Under this one, a compromised Warpgate can request a cert for any role its Vault token is allowed to sign for, and role defaults to one shared value across every target unless each one is individually configured otherwise. Certs also aren't revocable today, no KRL, no rotation path in this diff. None of this shows up in a line-by-line read, because every line does what it says, it's a property of what the whole system ends up guaranteeing. Posted a concrete proposal for this as a follow-up comment.

One more thing worth knowing before this merges: #2185 also adds Vault integration (a different problem, relocating static secrets into KV rather than issuing certs, but it collides mechanically with this PR in several places, workspace crate registration, Services, ConnectionError, SSHTargetAuth), and its TLS/mount-configurability work already solves two gaps in this PR's own Vault client. Posted the details as a comment on that PR, but flagging it here too since it affects how and when this one should land.

This doesn't mean the direction is wrong. Ephemeral, non-stored credentials is the right fix for a real, long-standing gap, and the mechanics here are solid.

Opened #2400 with a concrete design for the authorization question from the review above, rather than posting the whole thing inline here.

Short version: the core piece there, identity-templated Vault roles plus per-session scoped child tokens, so Vault verifies the principal instead of trusting what Warpgate asserts, belongs in this PR before merge, not a fast-follow. Without it, this design can plausibly have a worse worst-case blast radius than what it replaces (fleet-wide, non-revocable access versus today's bounded-to-stored-credentials), so it's not a good candidate for shipping as a documented limitation. The remaining hardening in the issue (full IdP-verified non-repudiation, host-binding, revocation) is genuinely separable follow-up work once that baseline is in.

@theredspoon Thank you for the follow-up review!
I've addressed the three technical points in commit bc0fb04c:

1. AWS Test Fix: Added AWS_SESSION_TOKEN to test_aws_signs_the_global_endpoint_by_default's test environment so it tests global endpoint signing without triggering StaticCredentialsDisallowed.
2. SSH Terminal Error Sanitization: Added client_message() to VaultError, AwsError, and ConnectionError. Full error bodies and Vault topology details are now strictly logged server-side (tracing::error!), while SSH client terminals receive safe, generic messages ("Target connection failed: Vault denied the certificate signing request").
3. Payload Validation in Stub: Added request-shape presence checks for AWS (iam_http_request_method, iam_request_url, iam_request_body, iam_request_headers), Azure (jwt, subscription_id), and GCP (jwt) login paths in stub_vault.py.

Went through the current head (bc0fb04c) again. Two new issues that weren't caught in earlier rounds, plus a few smaller items.

X-Vault-Token can leak on redirect

warpgate-vault/src/client.rs:94 builds the reqwest::Client with no redirect policy:

let http = reqwest::Client::builder().timeout(config.timeout).build()?;

That leaves reqwest's default policy in place, which follows redirects and only strips Authorization/cookies/proxy-auth headers on a cross-origin hop. It has no concept of X-Vault-Token as sensitive, so a 307/308 from the sign or unwrap endpoint (compromised Vault, misconfigured proxy, or MITM) replays the token to a different host, or moves an HTTPS request to HTTP. Please resolve with redirect::Policy::none() on this client and a regression test asserting the token is never forwarded cross-origin.

Unbounded buffering + panic in error-body truncation

warpgate-vault/src/client.rs:354-367:

let body = response.text().await.unwrap_or_default();
let max_len = 256;
let body = if body.len() > max_len {
    format!("{}... (truncated)", &body[..max_len])
} else {
    body
};

response.text() buffers the entire body before the length check runs, so a hostile or misbehaving endpoint can force unbounded allocation. Separately, &body[..256] panics whenever byte 256 falls inside a multi-byte UTF-8 character (255 ASCII bytes followed by é, for example). This is reachable from anything answering as the configured Vault address. Please resolve by streaming a bounded prefix and truncating at a char boundary, or truncating the already-bounded raw bytes lossily.

Smaller items

• read_credential() (client.rs:338-348) zeroizes the buffer it reads into, but returns a fresh, non-zeroized trimmed copy that then gets copied again into the JSON login body. The cached Vault token is correctly zeroized; the K8s JWT / AppRole secret ID / wrapping token read from disk are not. Please resolve end-to-end: zeroize trimmed and the JSON copy too, not just the initial read buffer.
• tests/stub_vault.py:61-82: the Azure login check only requires jwt + subscription_id, not resource_group_name/vm_name/vmss_name; the AWS check only verifies four fields are truthy without decoding or checking the request is actually GetCallerIdentity. Some of this is covered by assertions elsewhere in the integration tests, but the stub itself validates less than its shape suggests.
• Azure/GCP metadata_address is administrator-configurable and blindly GETed (warpgate-common/src/config/mod.rs:458 / metadata::gcp_identity_token), so a compromised config file can trigger SSRF from the Warpgate host. This doesn't cross a privilege boundary on its own since editing warpgate.yaml already requires host access, but it should get a line in the docs.

@theredspoon both fixed, thanks.

Redirects are refused outright now, which covers the metadata calls too. The error body is read chunk-wise with a 256-byte cap and truncated lossily, so a split character can't panic it. Chasing that one, I found the success path had
no bound at all — a 200 MB signed_key took a live gateway from 74 MB to 680 MB RSS, per session in flight.

Zeroization is end-to-end now: the login body goes through typed structs instead of a serde_json::Value, so there's no stray copy of the JWT or secret ID left around. The one remaining is reqwest's own send buffer, which I
commented rather than pretended about.

The stub validators actually validate now — decoded AWS payload, full Azure coordinates, JWT shape, GCP audience — and have tests of their own. You were right that they were asserting nothing.

A pass over the rest turned up a few more: lease_duration: 0 was read as expired, so every request re-logged in; nothing was checked about the returned certificate, so a host cert or one over a key we don't hold both went on the
wire; a comma in the target username widened valid_principals. Also added an optional certificate_ttl.

One I'd like your view on: a role with default_critical_options can put a force-command in the cert, and the target runs that instead of what the user typed. I made it warn rather than refuse — a restricted role might set one
deliberately, and a hostile Vault has target access anyway. If you think the stealth is the point, I'll make it refuse behind an opt-in.

425fb05. metadata_address is in the docs now. OpenBao offer still very
welcome.

Confirmed everything in dd061726 — redirect refusal, the chunked/lossy truncation fix, the response-size cap, lease_duration: 0 handling, the certificate type/key checks, and the comma-in-principal validation all hold as described. Ran the focused test suite too, all passing.

On critical_options: refuse by default, gated behind an explicit per-target opt-in.

The "hostile Vault already has target access anyway" framing undersells this. force-command isn't a subset of what a fully compromised Vault could already do directly: it runs under the connecting user's own principal and key_id, so it launders attribution in the target's own sshd log in a way a direct malicious connection never would. There's also a lower-privilege path than full Vault compromise: Vault's ACLs separate write access to ssh/roles/* from sign/* and from actual network reachability to targets. Someone with only role-config write, no signing rights and no path to the target, could plant a default_critical_options on a role and wait for a legitimate session to carry it through Warpgate. That's a materially lower bar than the #2400 threat model, and Warpgate is the only place that check can land.

The realistic case day to day is more mundane than either: a legitimate, uncompromised Vault, an operator who copies or templates a role with default_critical_options set, and a connecting user who gets no signal at all beyond a warn-level server log they're not watching.

Suggest: default-reject any critical option. Per-target opt-in as a named allow-list of expected option keys, not a bare boolean, and for force-command specifically, pin the exact expected command where practical rather than accepting any value. A rejection should reach the user the same way the certificate-mismatch errors do now, not just the log.

Two more, from this round:

lease_duration can panic Warpgate. warpgate-vault/src/client.rs:348-350:

expires_at: (auth.lease_duration > 0).then(|| {
    Instant::now() + Duration::from_secs(auth.lease_duration).saturating_sub(TOKEN_EXPIRY_MARGIN)
}),

lease_duration is an untrusted u64 straight from Vault's response, no upper bound. Instant + Duration panics on overflow. A misbehaving or compromised Vault returning an oversized lease crashes the process, on every login path. Please resolve with checked_add, rejecting an unrepresentable lease as an API error rather than crashing on it.

IPv6 loopback is misclassified as insecure. validate_address (client.rs:38) checks host == "::1", but url::Url::host_str() returns "[::1]" with brackets for an IPv6 host, confirmed by compiling and checking directly. A genuine loopback IPv6 Vault address (http://[::1]:8200) gets rejected as insecure the same as a real remote HTTP address would. Low severity, but a real bug for anyone running Vault dev-mode over IPv6 loopback.

One more, lower priority: the AWS path is the one exception to end-to-end zeroization. StsIdentityRequest.headers (an ordinary HashMap<String, String> carrying the SigV4 signature and session token) and the base64-encoded strings built from it in aws_login_body() are never wrapped in Zeroizing, unlike the Kubernetes/AppRole/Azure/GCP/token paths. Worth closing for consistency, not urgent given these are temporary credentials rather than static keys.

Ran a wider architectural sweep across the codebase, not just this PR's diff, then went back and verified every proposed fix against the real code and this PR's own existing patterns.

Certificate minting via the host-key-check admin endpoint

warpgate-admin/src/api/ssh_connection_test.rs's connection-test handler returns to its own caller as soon as HostKeyReceived fires, but the underlying RemoteClient task doesn't get that signal and falls through to authenticate_session regardless (warpgate-protocol-ssh/src/client/mod.rs:824). For a Certificate-auth target with an already-trusted host key, that mints a real cert and opens a real authenticated session in the background. Measured directly with a throwaway integration test: the session holds for a minimum of 310.6 seconds (the 5-minute inactivity timeout plus a 10s pad), indefinitely if ssh.keepalive_interval is configured, and the task itself leaks on every press, since create() discards its JoinHandle (client/mod.rs:361) and the read loop never exits on this path, so Drop for RemoteClient never runs. The throwaway session UUID is never registered via register_session, so certificate_key_id() falls back to warpgate:<random-uuid> with no username. (First press against an untrusted host key self-cleans in ~1ms, since HostKeyUnknown fires after the admin loop already broke on HostKeyReceived — it's every press after the key is trusted that holds.)

The fix needs to be a deterministic signal, not a race. abort_rx/abort_tx exist, and having the task treat a dropped abort_tx as cancellation is safe for real sessions (ServerSession's Drop impl explicitly sends the abort signal first, so there's no legitimate case where a real session drops it while still wanting the connection) — but tokio::select! is unbiased, and authenticate_session is called inside the same fut_connect arm that resolves concurrently with HostKeyReceived firing mid-KEX, so a drop signal alone still loses the race roughly half the time. Please resolve with an explicit intent passed from ssh_connection_test.rs into the connect command (a stop_after_host_key flag or a dedicated command variant) that makes wait_for_connection return before authenticate_session deterministically, not conditionally on a race. The dropped-abort_tx handling is still worth adding as defense in depth alongside it.

Separately: create() should hold and abort its JoinHandle instead of discarding it, but scoped to the admin-side caller specifically. Applying it to the shared RemoteClientHandles type would hard-abort real sessions instead of letting ServerSession::drop's existing graceful disconnect() run, losing a clean Disconnect::ByApplication on the target side.

Vault config doesn't hot-reload

services.rs:80-86 builds VaultClient once from the startup config snapshot into vault: Option<Arc<VaultClient>>. Every other config section hot-reloads through the watch::Sender in config.rs's reload path; Vault was never wired in. VaultConfig/VaultAuth are missing PartialEq/Eq (all fields are String/PathBuf/Option/Duration, so deriving both is straightforward, and ListenerParams already does the same for the same reason). The swappable-cell mechanism this needs already exists in this codebase: warpgate-core/src/rate_limiting/swappable_cell.rs's SwappableLimiterCell, built on watch::Sender<Option<T>>, documented as "a cell containing a reference which can be swapped out wholesale." One ordering constraint: Services::new runs before watch_config is called (run.rs:83 vs :122), so the rebuild loop can't live inside Services::new — it needs to be spawned from run.rs after watch_config, the same place the existing ListenerSupervisor gets spawned, following that same diff-and-rebuild shape (listener_supervisor.rs:132-171), keeping the old client on a validation failure the same way it keeps the current listener. Please resolve, editing or removing the vault: section currently has no effect until a restart.

Cloud metadata tokens can transit an ambient proxy

VaultClient's single reqwest::Client (client.rs:202-205, comment at :200 says "the same client fetches cloud metadata") is used both for real Vault calls and for Azure/GCP metadata-token fetches, with no explicit proxy policy, so reqwest's default of honoring HTTP_PROXY/HTTPS_PROXY/NO_PROXY applies. Both metadata defaults are plain HTTP (169.254.169.254, metadata.google.internal); GCP's is a hostname, which a typical IP-based NO_PROXY list won't match. Please resolve with a second metadata_http: reqwest::Client built with .no_proxy(), used only for the two metadata call sites, leaving the main Vault-address client's ambient proxy support untouched. AWS doesn't go through this client.

key_id enforcement and error messages

Checked against Vault's actual server source (calculateKeyID, builtin/logical/ssh/path_issue_sign.go, unchanged since v1.4.0): a role with allow_user_key_ids=false only falls back to the token's display name when the caller sends no key_id at all. When a non-empty key_id is sent and the role doesn't allow it, Vault returns an error and issues nothing. Warpgate always sends a non-empty key_id (client/mod.rs:841), so a misconfigured role already fails closed today, there's no silent wrong-attribution here, and a client-side key_id check would be unreachable.

Three real things remain from that investigation:

• The error Warpgate surfaces for this case is the generic "Vault denied the certificate signing request." Please resolve by mapping the specific setting key_id is not allowed by role response body to a message that names the fix (allow_user_key_ids=true on the role).
• certificate_mismatch() doesn't check valid_principals. Vault returns the requested principal set verbatim (trimmed, deduped, sorted) or hard-errors, never silently widens it, independent of the key_id question above. Please resolve by adding that check, using certificate.valid_principals().iter().any(|p| p == principal) rather than exact equality, since Vault sorts the returned set.
• Found separately while checking this: a Warpgate-side refusal (the existing cert-type/pubkey mismatch checks, or the principals check above) currently surfaces as ConnectionError::Authentication, whose client message is "SSH target rejected Warpgate's authentication request" — inaccurate, since the target never received anything in that case. Please resolve with its own error variant.

Response-wrapped AppRole secret IDs need the unwrapped value cached

login_body() re-reads and re-unwraps the same file on every login (client.rs:375-379), but wrapping tokens are single-use, so every login after the first fails, surfacing only as the same generic "Vault denied the certificate signing request."

Response wrapping protects one-time delivery of the secret ID, it doesn't force single-use of the secret ID itself. secret_id_num_uses/secret_id_ttl separately govern how many times the unwrapped secret ID can authenticate, and HashiCorp's own AppRole guidance for long-running services is to unwrap once at initialization and reuse the result for subsequent logins until it expires. That matches this PR's own README, which already describes the intent as "the secret ID is read fresh on every login, so it can be rotated underneath a running Warpgate", rotation as something available on demand, not required before every login.

Please resolve by caching the unwrapped secret ID, keyed on the raw file content, reusing it while the file is unchanged and only re-unwrapping when the content actually changes (an operator writing a fresh wrapping token). Keep a distinct error for the real failure case, an unwrap attempt (first use, or after a detected change) that fails because the token is stale or already consumed: VaultError::SecretIdUnwrap { path }, naming the file and stating that the provisioning process needs to write a fresh wrapping token, e.g. via vault write -f -wrap-ttl=<ttl> auth/approle/role/<role>/secret-id.

Lower priority

• A target with an empty username substitutes the connecting Warpgate user's own username as valid_principals. Not a bypass, Vault's allowed_users still rejects anything out of policy, but please resolve by documenting this mode in the README alongside the existing allowed_users guidance.
• default_extensions on a returned certificate are neither checked nor logged, while critical_options now are. Please resolve by logging default_extensions the same way, for the same operator-visibility reason critical_options was.

Both rounds are in commit 409e2e5.

@theredspoon
Before the details: your analysis and recommendations are worth more than everything I put in this PR. The code and tests were the easy part; what you did was find the things that made them wrong. Without your reviews this would
have shipped as one continuous hole with a feature description on top, not as an improvement. Twelve findings across the rounds, and the two most serious - the host-key check minting certificates, and the AppRole path that broke on
every login after the first are ones no amount of testing my own diff would have surfaced, because I was testing the diff and you were reviewing the system it landed in.

On critical options you changed my mind. "A hostile Vault already has target access" conflated two different capabilities: force-command isn't extra access, it's laundered attribution, and the target's own log is the thing this feature exists to make trustworthy. The role-write-without-sign path settles it. So: default-reject, per-target allow-list of names with optional pinned values, and the refusal reaches the connecting user rather than a log nobody watches.

Everything else landed as you described it checked_add on the lease, url::Host for IPv6, Zeroizing on the AWS path, the allow_user_key_ids message, valid_principals checked with any not equality, the unwrapped secret ID cached against file content, a VaultCell rebuilt from run.rs beside the listener supervisors, and a separate no_proxy client for metadata.

Two places I'm weaker than I'd like, said plainly:

The host-key check I took the explicit-intent route, a dedicated RCCommand::CheckHostKey that returns before authenticate_session, final hop only. What I can demonstrate is the leak: revert it and my test fails on connections still open after the request returned. What I could not reproduce is the certificate actually being minted the leaked task stalls before signing in my setup, over a 5s window. That assertion is a guard, not evidence; your 310.6s measurement is the real data point. If you can share how you drove it to sign I'll make it deterministic.

The JoinHandle I didn't thread one through. CheckHostKey ends the task, and the admin caller sends an explicit abort afterwards, scoped so ServerSession's graceful disconnect stays untouched. Two mechanisms rather than the third you
suggested; say the word and I'll add it.

Tests are 15 Rust unit and 57 integration, up from 12 and 48. Each new one was verified by breaking the code it defends including one that didn't fail on the first attempt, the valid_principals case, which rejects that certificate too. Rewritten to assert who did the refusing.

Pushed d818090, rebased onto current main.

This round came from building the test infrastructure rather than from reading the diff again. tests/vault_server.py runs the suite against a real Vault and a real OpenBao, reading requests back out of the server's own audit device, so
assertions are on what the server received rather than on what our stub chose to remember. Three defects fell out:

• Every login left a copy of the credential in freed memory. serde_json::to_string grows its String as it writes and frees each smaller buffer unwiped; Zeroizing only wipes the one that survives. Only shows at realistic sizes measured with a 4 KiB credential, which is what a K8s service account token actually is.
• The certificate's key ID was never checked against the one requested. A 64 KiB key ID authenticated normally, which quietly breaks the attribution this whole path exists to provide.
• The reason an authentication failed never reached the user — it went to the server log only. For a certificate outside its validity window, the documented clock-skew case, that sends someone to debug credentials that are fine.

Also OpenBao refuses to enable an audit device over the API, and its config stanza needs type, path and an options block — a top-level file_path is accepted with a warning and then ignored. Documented, since the issuance record on the Vault side is half the point.

Two CI gates are red and neither is from this branch:

• biome fails on AuthPolicyEditor.svelte, which came in with 55af452 and is byte-identical here.
• cargo-deny fails on RUSTSEC-2026-0253 (lru via ratatui), published after main's last green run. deny.toml already carries RUSTSEC-2026-0002 for the same crate with "no update available".

I left both alone rather than touch unrelated files in a security PR.

Pushed 6bd00e1. Three more defects, found by reading other projects' advisories and by pointing two tools at this code that had not been used on it before.

A certificate naming more than the target account was accepted. The check asked whether the requested principal was among those returned. Vault returns the requested set verbatim or refuses, so anything extra means the answer did not come from this request and each extra name is another account the target will accept the certificate for, chosen by whoever answered rather than by the operator. Under AuthorizedPrincipalsFile it need not resemble a username at all. Now required to be exactly the account asked for.

This came out of two advisories rather than out of the diff: CVE-2024-7594, where an empty valid_principals yielded a certificate good for any user on the host, and CVE-2026-35414, where a comma inside a principal splits one name into two for one of sshd's checks and not the other. The second is also why the rule is "exactly one name" rather than "contains" it notes the attack works when the CA does not reject commas in what it is asked to sign, which is the check on the request side that was already there.

A certificate could write escape sequences to the connecting user's terminal. The refusal message quotes the critical option's name straight out of the certificate and is printed to the PTY, so a name containing \x1b[2J cleared their screen rather than appearing in the text. Certificate-derived strings are now quoted with {:?}.

The outbound SSH handshake had no bound of its own. A target that completes the TCP connection, sends a valid identification string and then goes silent held the gateway's task, socket and session slot until the inbound session's inactivity timeout fired measured at 55s with that timeout set to 45s. That setting governs how long an idle interactive session may live and is legitimately raised to hours, every one of which extended this hold to match. Bounded now by a dedicated 30s deadline, with an error that names the stage.

tests/hostile_ssh_server.py is new and needs no Docker: six ways of being a bad SSH server. The rest of the suite treats the target as honest, which is the one trust boundary nothing here had pushed on and russh, which Warpgate is the client half of, has published pre-authentication panics reachable from the peer. Five of the six modes were survived without change; russh bounds the identification string itself, which covers two of them.

cargo mutants found the fourth problem, in the tests rather than the code: it replaced the error-body reader with one returning an empty string and everything still passed, because the assertions were all upper bounds. Ten mutants survived in that one function.

Checked and clean, for the record: russh 0.62.6 is current against all fourteen of its advisories, and allow_insecure_algos keeps the strict-KEX extensions, so the Terrapin mitigation is not lost in the mode meant for older devices.

CI is still red on biome and cargo-deny, and neither is from this branch the same two are red on #2409 and #2410. AuthorizedPrincipalsFile.svelte came in with 55af452 and is byte-identical here; RUSTSEC-2026-0253 (lru via ratatui) was published after main's last green run, and deny.toml already carries RUSTSEC-2026-0002 for the same crate.

Ran a final-gate pass with three independent reviewers plus direct verification against real sshd servers, since this round changed enough surface (the real-Vault/real-OpenBao test harness, the critical_options allow-list logic, the CheckHostKey command) to be worth a genuinely fresh look rather than re-confirming what's already fixed. Everything from the last round not mentioned below has been confirmed separately. Two real, previously-unflagged issues, plus a cluster of smaller ones.

Host-key check returns the wrong key for any target behind a jump host

Already independently reported and being fixed: issue #2412 and its open fix, PR #2413 (resolve_ssh_chain tags each hop with its target_id, and the admin endpoint now waits for the specific hop being asked about rather than breaking on the first HostKeyReceived it sees). No need to duplicate that here.

What #2413 doesn't cover, since it's written against main and this PR's stop_after_host_key/CheckHostKey gating doesn't exist there yet: whether an intermediate hop's own authentication is actually prevented when only that hop's key is being checked. Right now stop_after_host_key is gated on is_last/hop_count, not on which target the caller asked about, so once #2413 lands and this rebases onto it, the same target_id tagging needs to also drive the stop_after_host_key decision, not just which key gets returned. Otherwise a caller checking an intermediate hop's key specifically (which #2413 makes possible to do correctly) still doesn't stop that hop from authenticating.

Related to that: no certificate gets minted for the jump host today, but that's not a construction guarantee the way it is for the final hop, it's the admin endpoint's abort winning a race against the SSH handshake, the same category of fragility CheckHostKey exists to eliminate. Confirmed empirically (40 consecutive presses, both host-key-known and unknown, 0 Vault sign requests, 0 userauth attempts logged), so not currently exploitable, but nothing structurally prevents it and there's zero test coverage for any chain longer than one hop.

Pinned critical options are only checked when the certificate actually carries them

certificate_mismatch()'s critical-options loop (client/mod.rs:167-185) iterates over options present in the returned certificate and checks each is allowed, correctly rejecting anything unexpected. It never checks the other direction: that a target's configured, pinned options are actually present. A target configured with a pinned force-command accepts a certificate carrying no force-command at all, no restriction, full shell. This is exactly the threat model already established for this feature, someone with Vault role-write but not sign-rights, just approached from the other side: instead of adding an unexpected option, they remove an expected one, and nothing here catches it. Please resolve by checking that every entry in the target's allowed_options with no value pin, or a specific value pin, is actually present in the returned certificate, not just that nothing extra showed up.

Smaller items, roughly by severity

• warpgate-vault/src/metadata.rs's Azure (.json(), lines 40-52 and 54-66) and GCP (.text(), lines 78-92) calls buffer the full response with no size cap and no zeroizing of the intermediate buffer, the exact defect class this round's own read_json fix (client.rs, MAX_RESPONSE_BODY) closed for the main Vault client, just not mirrored here. Reachable on every Azure/GCP relogin.
• LOGIN_PAYLOAD_CAPACITY (client.rs:118, 32KiB) is a pre-allocation hint, not a bound. read_credential doesn't cap what it reads from token_path/secret_id_path. A credential file larger than 32KiB reintroduces the grow-and-copy leak this round's zeroization fix was built to close, silently.
• certificate_mismatch()'s valid_principals check verifies the requested principal is present, not that nothing else is. Already fixed in 6bd00e187, pushed while this review was in progress: now requires an exact single match rather than containment, stronger than what this would have asked for, backed by two real CVEs (CVE-2024-7594, CVE-2026-35414) that make this a better-documented severity than "low, needs a fully rogue Vault."
• No check anywhere on the certificate's own validity window. tests/test_vault_hostile_certs.py:112 references SECURITY_TESTING.md to explain why this is accepted risk; that file doesn't exist in the repo. A misconfigured or compromised Vault role can hand back a certificate valid for years, unnoticed, bounded only by the role's own max_ttl, undocumented in the README's certificate_ttl section. Worth noting: Add Ssh cert auth for targets #1847, the alternative self-hosted-CA implementation of this same feature, defaults its own issued certificates to a 1-minute window, so short validity is already the project's own expectation, just not enforced on the receiving end here.
• CI can go green with neither real issuer actually tested: a Docker image pull failure calls pytest.skip() (tests/vault_server.py:77) rather than failing the run, and the OpenBao image is pinned to mutable latest. This is a new pattern for this repo specifically, not an existing convention: nothing else in the test suite skips on infrastructure failure, .github/workflows/test.yml builds every other test image in an explicit step that fails the job outright on error. Given the harness is meant to be the contract gate, worth making pull failure a hard failure and pinning both images to a digest.
• The AWS SigV4 headers are copied unwiped at least three more times beyond the serde_json::to_string line already flagged, warpgate-aws/src/sts_identity.rs's Credentials, the Identity it's moved into, and the http::Request header map apply_to_request_http1x writes the session token into. Worth scoping that fix to the whole AWS path rather than the one call site.
• Two contract tests don't test what they claim: tests/test_vault_contract.py's key-ID test supplies a truncated/malformed public key that both real Vault and OpenBao reject at parse time, before allow_user_key_ids policy is ever consulted, so it passes without exercising the thing it's named for. Separately, server.signs (tests/vault_server.py:282) reads only the audit device's request entries, so the test asserting the certificate carries the right principals re-checks what Warpgate sent, not what either real server actually returned.
• Sub-second certificate_ttl values fail only at connect time (Duration::as_secs() truncates to "0s", both real issuers reject it), not at config load. Worth validating at config time instead.
• Minor UI bug: clearing a pinned critical-option value in the admin UI (Options.svelte) writes back an empty string rather than clearing the pin, so an operator who types a value and deletes it gets an exact-match-empty pin instead of "any value" as the placeholder implies. Fails closed, but the UI says the opposite of what happens.
• warpgate-vault/tests/zeroization.rs:123-144 has orphaned, truncated doc comments describing tests that were deleted, and login_payload is private, so the suite's own safety-net tests reimplement the safe pattern inline rather than exercising the real function, reverting the actual fix would leave every assertion in that file green.
• .github/workflows/docker.yml's IMAGE_NAME change to ${{ github.repository }} looks like unrelated fork scaffolding inside a security PR, worth dropping from the branch.

One more, separate from the above: the terminal-escape-sequence fix in 6bd00e187 looks like it's the first time this codebase has addressed that bug class at all, and there's no shared sanitization helper anywhere for it. warpgate-protocol-ssh/src/server/service_output.rs, command_detector.rs, and session.rs:584 all write untrusted-ish strings toward a PTY too; worth a pass to check whether any of them have the same exposure now that the pattern's been named once.

Given how many of the above are tests passing without exercising what they claim to, worth doing your own adversarial pass over the test suite specifically, not just the production code, and writing down whatever gaps that turns up so they don't quietly regress later.

Rebased on current main (10 commits merged, two conflicts resolved by keeping both sides).

Since the last push this branch went through six rounds of external review by @theredspoon - including one pass with three model providers in parallel - plus our own adversarial agents and a two-provider arrangement where one
side wrote code and the other ruled on whether the evidence was sufficient.

115 distinct defects were raised. 109 are fixed with evidence, 2 are tracked upstream because they are upstream's code, 3 were withdrawn, and 1 falls outside the project's published threat model. Nothing is deferred.

One of the two upstream items was the web-SSH host-key storage bug, reported separately and fixed by @Eugeny in fb66ff7; the fix is merged here.

What the evidence is

tests/mutation_matrix.py disables each of 47 security checks in turn and requires that the test named after that check is among the failures — not merely that something failed. That distinction matters here because the
integration suite runs against a real sshd which independently refuses expired certificates, unknown principals and unrecognised options, so a failed connection proves nothing about our code.

46 of the 47 guards are measured discriminating on the tree as it stands. The artifact is tests/verify-guards.json, stamped with the commit it was produced from.

The forty-seventh is connection: the handshake deadline resumes after a host key answer. Its named test passes with the guard disabled, which means the test is not evidence for that guard. The cause is not yet established - the anchor is unambiguous, the pause and resume both execute on that path, and only the Ignore verification mode short-circuits it - so it is being measured rather than guessed at. Stating it here rather than rounding it up to 47.

Notable fixes in this round

• The signing CA can now be pinned (vault.ca_public_key). Every other check on Vault's response asked whether the certificate matched the request; none asked who signed it.
• A username can no longer collide with the gateway's own attribution in the certificate key ID. attribution() puts admin-token in that field, and five of the six paths that create a user did not refuse the name.
• The handshake deadline is paused for a host-key decision and resumed afterwards, rather than disarmed.
• The Vault token mutex is no longer held across an unbounded operation.
• A Vault role is validated when a target is saved, not only when a session
  tries to use it.

Happy to walk through any of these, or to share the full review ledger and audit report if that would help review.

Left

Two minor fixes need to be done. I'll finish them tomorrow

@theredspoon - every item from your last round, numbered as you numbered them, with the commit that closed it. Three of the twelve were sharper than they looked and one of them was a real memory-disclosure defect I had argued against before measuring it; that argument was wrong and the measurement settled it.

Where the evidence stands, and what you can check from the PR alone. 47 of 47 guards measured discriminating, in one run over one commit. That measurement was taken before the merge above; the eight guards whose code the merge touches are being re-measured, and I will give the two numbers separately rather than one figure for both trees. After the merge: all 47 anchors present, unit tests 56 + 27 + 4 passing, clippy deny set clean.

Worth being straight about what that claim rests on, since you cannot verify it from this diff. tests/mutation_matrix.py is in the PR, so the 47 guards and the test named for each are there to read, and its refusals are there to trigger. The A/B runner that produced the number is not, and neither is the artifact it writes: they belong to how we worked rather than to what Warpgate does, and putting a tool in a PR because it justifies the PR is the wrong reason to ship code. So the number is our report, not something you can reproduce from what is in front of you. If you want either the runner or the run's output, say so and you get them.

Thank you for the round. Several of these were things we would not have found, and I-10 is one we had actively argued against.

@janisdombr Great teamwork so far, thanks for working on this item with me.

Ran the mutation matrix myself rather than working from the report, all 47 guards, two full runs plus a third targeted rerun on anything the first two disagreed on. Tool issues first, then results, then asks.

Two real issues in mutation_matrix.py itself. Please resolve both. A literal duplicate entry in DISCRIMINATES ("admin: a Vault role the signing path would refuse is refused on save", byte-identical both times) trips the script's own duplicate-entry check, so as committed it refuses to run at all. Separately, the documented invocation (python -m tests.mutation_matrix, no flag) reruns the entire three-file suite plus every crate's full test binary per guard; the fast, targeted A/B behavior your report describes is behind an undocumented --named flag, which needs to actually be documented as the real invocation.

One correction, no action needed: "the A/B mutation-testing runner... is not in the PR" isn't accurate. mutation_matrix.py is a complete, working A/B runner, I ran it. What's actually gone is a different, separate file (tests/verify_guards.py/verify-guards.json, deleted in an earlier commit). The current artifact path is tests/mutation-matrix.json.

Results, all 47, and what each group means for you

35 discriminate cleanly, stable across every run. No action needed on these.

11 guards' tests failed because of test-harness fragility, not application bugs. Please resolve both root causes.

1. Hardcoded localhost resolves to ::1 in some Docker networking setups, this one included, where only 127.0.0.1 answers. Affects make_user_and_target (tests/test_ssh_target_cert_auth.py:113) and everything built on it, including target_on() (tests/test_vault_hostile_target.py:79-82). Pin to 127.0.0.1 explicitly.
2. The ssh-keys bind mount isn't routed through a configurable path. ProcessManager.start_ssh_server (tests/conftest.py:234) does -v {os.getcwd()}/ssh-keys:/ssh-keys, relative to wherever the repo is checked out. A Docker setup requiring explicit directory allow-listing gets a silently empty mount, sshd has no host key, every target container fails to start. Route it through the same configurable tmpdir mechanism the rest of the fixtures already use.

All 11 confirmed directly, not guessed: fixing both turned every baseline-red and merge-touched guard green; for the 3 previously reported as "does not discriminate" or unstable across runs, re-running the guard's own mutation against a rebuilt binary with the fix applied gave a clean, deterministic A/B result. None of these are application defects, and none of the 3 misreported ones are real coverage gaps.

One real ask that fell out of chasing the sandbox issue. warpgate-protocol-ssh/src/client/error.rs:51 maps every russh::Error, including a plain connection-refused, to the identical "SSH protocol error" string on the admin API. That's fine for an SSH client mid-session, sanitizing detail from an untrusted party makes sense there. It's not fine for the admin host-key-check endpoint specifically: the caller there is an authenticated operator who needs to know whether "I can't reach this jump host at all" and "this jump host's key is untrusted" are the same problem or two very different ones requiring different responses.

Please resolve by giving that endpoint's response a way to distinguish connection-establishment failure from an actual host-key rejection, even if the generic message stays for every other caller.

The rest of the round, verified directly against code rather than the write-up

Five confirmations, no action needed:

• Admin host-key-check address-match: replaced by chain-identity matching (role() vs check_target), stronger than the write-up's own claim.
• Admin check-host-key persist race (I-3): no race — HostKeyReceived fires before known_hosts.validate is ever called.
• Cargo.lock/thiserror: clean, cargo check --locked succeeds.
• I-12 regression and fix (39bde2a8f, 37e1c34d8, 4bc403677): confirmed both ways — reverted and reran, failed as expected, restored.
• Order-dependent CI test: a shared module-scoped fixture was leaking trust state; the fix gives it independent servers.

Two asks:

• 821e60349's test doesn't cover what it claims to. Please resolve. The handshake-deadline-resume fix itself is real, genuine pause-and-restore rather than the earlier disarm-and-never-rearm bug. But its only test is a unit test that checks two constants are internally consistent, it never calls the actual .reset() at the real call site. "Weaker than an end-to-end proof" undersells that, it's not close to e2e, it doesn't touch the call site at all. A future edit that calls the right function with the wrong value, or forgets to call it, would still pass.

Extend the test to exercise the real call site, not just the constants.

• Signing-CA pinning has no commit hash. Please add one. The fix itself is real and fail-closed on an unparseable pin (found in client/mod.rs), but it's the only item in the write-up without one, worth fixing for consistency with everything else being tracked.

Full reconciliation against our original 33-item review, everything raised since, and one earlier item — checked directly against code at the current head (0df7ae56).

Still open

Item 8: authentication_budget also bounds the target's own USERAUTH response, unbounded. Unchanged since first raised. Traced the actual math: max(30s, vault.timeout × 5 + 5s), and vault.timeout has no upper clamp in config — so a target that goes silent right after receiving its certificate holds the session, ephemeral key, and live certificate for a window sized for Vault's slowness, not the target's. Default config: 55s. A generous vault.timeout scales this with no ceiling.

Ask: give the target's own USERAUTH response its own, separately-bounded timeout, independent of vault.timeout.

Item 15/18: two small key-material hygiene gaps. key_id_field still does a plain .replace(':', "_") — root:admin and root_admin still collide, and the doc comment claiming "no real username can collide" is factually wrong about what the function does (it substitutes, it doesn't reject). Separately, VaultAuth::AppRole's role_id is still a plain String, unlike every sibling secret in the same struct.

Ask: either reject : in usernames the same way the admin API now does for locally-created ones, or use an encoding that can't collide (percent-encoding, or escape _ first). Wrap role_id in Secret<String> for consistency — Vault itself treats it as non-secret, so this is hygiene, not an active leak.

Item 19: 13 more fixed-name temp-file instances beyond the two originally fixed. warpgate-vault/src/client.rs (11 sites) and warpgate-vault/tests/zeroization.rs (2 sites) — all PR-added test code — still build host paths with only a process ID for uniqueness, several writing real credential material.

Ask: apply the same tempfile-based fix already used for the recordings path to these 13 sites.

Item 22: Vault error bodies and host strings both still reach logs unescaped, at 7 sites (one more than originally counted). Confirmed the mechanism precisely: every affected site uses %field/Display, and a \n in a Vault error body or an unresolved host string genuinely forges a log record in the default text format — confirmed by contrast against sites already using ?field/Debug, which does neutralize this. The I-9 PTY-sink fix doesn't help here; no tracing call routes through it. One more site than originally named: session.rs:436's emit_service_message debug-logs the raw message before the I-9 escaping runs. Separate good news: the key_id_field-colon part of this same finding turned out to already be covered — validate_key_id independently rejects control characters fail-closed before any key ID reaches Vault or gets logged, so that specific path doesn't actually forge anything.

Ask: switch the listed sites from %/{} to {:?} for Vault error bodies and host strings, matching the pattern already used elsewhere in this file.

Item 24: the never-expiring-certificate test is still skipped. Same skip reason as when first raised. Worth knowing even if unskipped: its current assertions (exit code + absence of one string) wouldn't check Warpgate's own refusal message anyway, so unskipping it as-is wouldn't fully close the original gap.

Ask: un-skip the test, and strengthen its assertions to check Warpgate's own refusal message (not just exit code) while doing so, so unskipping actually closes the gap rather than reopening a weak one.

Item 28: test_token_zeroizing still only tests the zeroize crate's own Deref, not any Warpgate code path.

Ask: delete it as redundant, or rewrite it to assert something Warpgate-specific.

Item 29: the hostile-option-name terminal test still lacks a positive anchor. Every sibling test in the same file already uses the pattern ("Warpgate refused the certificate" in shown); this one's the outlier.

Ask: add the same positive anchor the sibling tests already use.

Item 31 (second half): the admin host-key-check endpoint's error sanitization has no test at all. Bonus finding while checking: the nearest existing test that could plausibly cover this is actually non-discriminating for it — it asserts a string that also appears in an unrelated error variant's Display text, so it would pass identically with sanitization removed.

Ask: add a test that actually exercises this endpoint's sanitization specifically — asserting that raw internal error text can't reach the admin API response, using a case the existing non-discriminating test doesn't cover.

Item 33: the abort-branch ordering isn't actually fixed, and "harmlessly" undersells a real divergence. The _ matching pattern is correct now and documented. But Done still gets sent before the abort reason is returned — exactly the ordering the sibling HandshakeTimeout branch's own comment, a few lines above, explains why to avoid. Separately: the two sites that treat a dropped sender differently aren't equivalent-but-different, they diverge in kind — one would hang forever on that input (no live bug today, since every current owner sends before dropping, but not equivalent to the other site tearing down cleanly).

Ask: reorder so the reason is captured before Done is sent; either make the command-loop site match the same catch-all pattern, or document why it deliberately doesn't.

Mutation-matrix CI wiring: not in any .github/workflows/ file. Right now "guards discriminate" is true only at the moment someone runs this by hand and reports it — nothing stops a future change from silently breaking a guard's test coverage.

Ask: run it in CI (even just --named mode, given the full mode's cost) and fail the build on anything short of full discrimination. That's the durable version of "checkable without rerunning it" — a green check, not a committed snapshot.

Item 17 (residual): I-10's test guards the buffer-growth helper, not the real call site (already self-disclosed honestly). Plus two smaller residual unzeroized paths found while checking: error_body's 256-byte Vault error buffer, and read_bounded_json's serde_json scratch buffer (both low severity, bounded).

Ask: exercise read_bounded directly in the zeroization test, or extend the honesty note to name the call site explicitly. Zeroize the two residual buffers too, for consistency, even though the severity is low.

Doc nits: README's certificate_ttl table lead-in still says "Three cases" above what's now a four-row table. And the comment at client.rs:132-133 — "Revocation is still handled: a rejected token is dropped when signing comes back 403" — conflates cache invalidation with actually revoking at Vault; the reasoning for why that gap is fine (a 403'd token would likely fail revoke-self too) doesn't appear in the code, only in the write-up.

Ask: fix the README lead-in to say "Four cases"; rewrite the client.rs comment to describe cache invalidation accurately, and consider inlining the revoke-self reasoning so it's not only in this thread.

Commit attribution: two small mismatches (I-4, I-6) — content's right, just cited to the wrong commit (both actually landed in 110820c5, not 2a978f72c/d482ad7ae).

Ask: repoint the references if it matters for your own tracking — not required, content is correct either way.

One more precision correction. "A Vault role is validated when a target is saved" reads broader than what shipped. What's actually there is a syntactic name-format check, shared between the admin save path and the connect-time path — real, and a genuine improvement, but not a check that the role exists in Vault. warpgate-admin has no dependency on warpgate-vault and there's no role-lookup API to call one even if it did. A syntactically valid but nonexistent role still saves successfully and only fails at connect time — the original "fail-closed, reasonable follow-up" framing is still the accurate description.

Ask: narrow the claim to "the same name-format rule now applies at save time and connect time" — the current phrasing implies more than it delivers.

One more thing, unrelated to the code itself. An earlier comment on this PR (issuecomment-5271487696) included a process suggestion — a 'maker-check' pipeline (separate RED test-builder/verifier, GREEN implementer/verifier, and deterministic re-checks run by orchestration rather than folded into any subagent's own judgment, plus multi-provider adversarial review before merge). It hasn't come up since, so mentioning it again in case it got lost in everything else this thread covered — no pressure either way, just didn't want it to go unseen.

A recurring pattern across this review was tests that turned out not to exercise what they were named for — not a guarantee every one of them would've passed with the guard removed, but enough of a pattern that it seems worth naming. Some of that may be exactly what the pipeline above is aimed at. Some of it might just be the sheer size of this effort — a diff and a review this large probably pushes against context limits on both sides, which can cause the same kind of thing independent of process. Likely some mix of both, but wanted to flag that there's something structural here worth thinking about, beyond any individual finding.

@theredspoon Pushed — 46 commits, merged with v0.28.0, 0 behind main.

Twenty-one of your twenty-five asks are closed and no product code is open. But the thing worth your time is not in your list.

Un-skipping one test found a panic

Item 24 asked me to un-skip test_a_certificate_that_never_expires and strengthen its assertions. I did, and it failed — not on an assertion, on a client timeout. The gateway's log ended at "Certificate carries extensions" with no refusal, and then:

thread 'tokio-rt-worker' panicked at library/alloc/src/string.rs:2943:14:
a Display implementation returned an error unexpectedly: Error

humantime's Display returns Err rather than truncating for any time at or after the year 9999 (humantime-2.4.0/src/date.rs:267), and to_string() panics when a Display errors. The validity window is rendered for the diagnostic message above the call to certificate_mismatch. So ssh-keygen -V always:forever — and equally a Vault role with no TTL, or one with an absurd max_ttl — killed the worker mid-connection.

Three consequences, all observed rather than reasoned about:

• the guard that refuses a never-expiring certificate could never fire, because it sits after the line that panicked;
• the client was left holding a connection nobody would ever answer, which is item 8's class again;
• it is reachable by the issuer, which this design treats as hostile throughout.

Why nothing caught it: the unit test for that guard calls certificate_mismatch directly and never reaches the panicking line, and the one test that did reach it was skipped on the symptom the panic produced — "holds the session open for ~45s for a reason not yet isolated". A test disabled because of the defect it had found, parked for a week.

That is your closing observation, one level deeper than you put it. Fixed in 859fe990c, with a guard that drives the first second of the year 9999 and also asserts an ordinary expiry still renders as a date, so a fix that calls everything unrenderable fails it.

Four claims of mine were wrong

• Item 22's count. I said nine sites, "verified by grep". The grep required % on the same physical line as the macro and could not see multi-line tracing! calls. It is eleven. A check I chose, ran and reported on myself, which passed because it could not see the thing it was looking for.
• Why item 24 was skipped. I said the missing PTY meant the refusal had nowhere to go. Wrong — the task had panicked. The log said so; my reading of the code did not.
• "HACKED=x" in the terminal-escape test. ssh-keygen splits critical:NAME=VALUE on the first =, so the value is not part of the name the message quotes. Correcting it exposed the better point: "the name is present" and "no raw escape arrived" are both satisfied by a fix that strips the name entirely. The escape now has to appear in its inert form, \u{1b}[2J.
• A mutation-matrix run I reported as measuring something. It was killed, and a killed runner's exit code is indistinguishable from a caught guard. Recorded as not run.

Your list

Item 8 — the target's USERAUTH answer has a flat 30s bound of its own, applied at all five call sites so a new credential type cannot arrive unbounded by being forgotten. New TargetAuthenticationTimeout variant, so the party that went quiet is named.

Item 15/18 — key_id_field percent-encodes (% first, then :). Reading the comment you flagged as false turned up a second defect it was concealing: UNATTRIBUTED goes into the same field and user_key_id_field consulted only TOKEN_ATTRIBUTIONS, so a user named unattributed was indistinguishable from a session with no user recorded. Two lists of reserved names, one consulted. role_id is Secret<String>; config-schema.json is byte-identical, verified by regenerating.

Item 19 — thirteen sites, all through tempfile::TempDir. Item 28 — deleted; it tested the zeroize crate's own Deref. Item 29 — anchored, and corrected as above. Item 33 — the reason is captured before Done, and the command loop's Some(()) is now _; those were not two spellings of one thing, since a closed channel disables that branch rather than firing it.

Item 31 — unreachable_reason(kind) separates "cannot reach it" from "its key is untrusted", on the kind and never the OS string, because this is the sanitiser. It had no test at all; it has two, and the mutation for one restores e.to_string(), which is the bug the arm exists to prevent.

Item 17 residual — error_body's buffer is Zeroizing and reserved. read_bounded_json's serde_json scratch is not fixed and cannot be from here; recorded as a limit rather than closed. I-10's test guards the helper, not the call site, and now says so in its own doc comment — driving read_bounded would put hyper's buffers inside the canary detector's window, and a test that fails for a cause we cannot fix is worse than one that states what it does not check.

Doc nits done. And you were right to narrow the role-validation claim: what shipped is a syntactic name-format rule shared by the save path and the connect path, not a check that the role exists. warpgate-admin has no dependency on warpgate-vault — verified in its Cargo.toml — and there is no lookup to call. The write-up says that now.

Item I-4/I-6 attribution: you are right, both landed in 110820c5. Noted here rather than by editing the earlier comment.

The mutation matrix, in CI and measured

.github/workflows/mutation-matrix.yml: the guards whose anchor file a PR touched on pull_request, all of them on a schedule and on dispatch. A guard with no named discriminator fails the build.

With the honest cost attached, because it changes the recommendation. A full sweep is 14h20m on an M-series laptop. GitHub-hosted runners are 4 vCPU and slower, and the job limit is six hours, so the full sweep does not fit there at all. And the per-PR saving is smaller than it sounds: --changed against four commits selected 34 of 53 guards, because client/mod.rs carries two thirds of them. You may well want only the scheduled half, or a self-hosted runner, or the whole thing sharded — that is your call and I would rather hand you the number than the recommendation.

Six defects turned up in the instrument itself along the way. It would not start at all — its own duplicate check tripped on a duplicate that had been committed. Its documented invocation ran from nowhere. It paid the whole repository's collection cost for a single guard, in three separate places. It printed nothing for hours, so three runs were killed as hung. Two runs executed side by side rewriting the same files, because the lock was written rather than checked. And the mutated gateway leaked into the next guard's baseline, which voided 28 verdicts of a preceding sweep.

53 of 53 guards discriminate, measured in one run at 02792acfc after all six were fixed, tests/mutation-matrix.json carrying partial: false and refused: null. What that does not say: that these are the right guards, or that the set is complete. Different claims, neither made here.

Not done, deliberately

Upstream logs client-controlled strings through Display at about a dozen sites in warpgate-protocol-ssh/src/server/session.rs — exec commands, environment variable names and values, subsystem names, socket paths, a login name, and two "Target {} not authorized for user {}" lines interpolating the name from the client's own login string. Same mechanism as item 22 by a shorter route: no Vault, no admin access, just an SSH client sending an environment variable. Every one verified present on origin/main, so none of it is ours. It belongs upstream as its own change rather than widening a diff you have already called large — but I did not want to decide the scope of somebody else's security fix silently, so: your call.

Your process suggestion

It sat unanswered for six days and you were right to raise it again. It is now half-built, and not in the shape you drew.

Rather than RED and GREEN subagents inside one process, the split is across two providers with hard role separation: I build and operate checks, an independent auditor specifies them and rules, and neither writes into the other's territory. Deterministic checks are run by orchestration, as you said — that is what the matrix is.

It paid for itself in a way I would not have predicted. The auditor found the dirty-binary leak above — from a running sweep's output, not from reading the code. I had looked at the same two already failing lines in my own status notes and written "look into this later". Every other defect this round came from someone reading; that one came from someone watching. It is the strongest argument for the split that I have, and it is not the argument either of us made for it.

Everything green after the merge: 63 + 26 + 4 unit tests, 109 integration tests against a real sshd.